@azizur100389 is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

CI Report

✅ All checks passed

Pipeline Status

Test Results

✅ All 8017 tests passed

Code Coverage

Tests

_{📋 View full run · Generated by CI}

Claude finished @magyargergo's task in 6m 7s —— View job

PR #1296 Review — fix(storage): derive registry name from canonical repo root, not worktree slug

Tasks

• Read repo context files (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
• Read changed files (git.ts, run-analyze.ts, git-utils.test.ts)
• Read nearby relevant files (analyze.ts, status.ts, wiki.ts, index-repo.ts, ai-context.ts, repo-manager.ts, server-mapping.ts)
• Assess getCanonicalRepoRoot() implementation
• Verify runFullAnalysis() naming path and embedding server mapping
• Verify registry duplicate behavior
• Verify AGENTS.md / MCP URI behavior
• Assess test coverage and environment fragility
• Check CI/Docker status
• Compile findings and write final verdict

Review bar for this PR

• Analyzing from a linked worktree falls back to canonical repo root basename (not worktree slug) in ALL analysis paths, not only the early-return path.
• Explicit --name wins over all auto-derived names end-to-end (early-return, registry, embedding mapping).
• Remote-derived name wins over canonical fallback in all three paths.
• repoPath storage location stays intentionally worktree-local.
• AGENTS.md MCP URI generation uses the corrected project name from registerRepo, not an independent basename derivation.
• Embedding server mapping lookup key matches the final registry/project name in all cases (--name, remote-derived, canonical fallback).
• Tests use real git worktree add and prove the full analyze → registry → AGENTS.md symptom, not only the helper.
• CI, Docker/build, typecheck, and focused tests are green.

Issue and PR context considered

#1259 symptoms:

1. ~/.gitnexus/registry.json accumulates duplicate entries: one under canonical repo name, one per worktree slug.
2. AGENTS.md rewritten with the wrong worktree slug in MCP URI blocks (gitnexus://repo/wt-feature/...).
3. MCP context/impact queries miss the canonical index when addressed by worktree-slug name.

Changed files: gitnexus/src/storage/git.ts (+44/-0), gitnexus/src/core/run-analyze.ts (+19/-3), gitnexus/test/unit/git-utils.test.ts (+80/-0).

Current CI/Docker status: Both CI and Docker Build workflows are in-progress (not completed) at time of this review. A prior auto-generated CI comment showed ❌ Some checks failed with Tests: failure, 3 platforms at the stage level despite 7823/7823 tests passing — indicating a job-level failure (exit code / infrastructure) on all three cross-platform legs that must be resolved before merge.

Local test failure note: Author's Windows dev machine has os.tmpdir() inside a personal git repo, causing the "plain temp directory" test to fail locally. PR body asserts CI containers are unaffected.

Main risks: partial path coverage (early-return fixed, full analysis not), embedding/registry name divergence, registry duplicate entries from full analysis path, no integration-level regression test for the core symptom.

Understanding of the change

What #1259 reported: getGitRoot uses git rev-parse --show-toplevel, which inside a linked worktree returns the worktree's own root (e.g. /repo/wt-feature), not the canonical repo root (/repo). Downstream code derived the registry name from path.basename(repoPath) — the worktree directory slug — causing separate registry entries, wrong AGENTS.md MCP URIs, and split MCP query results.

Why getGitRoot is correct for some callers: cli/status.ts, cli/wiki.ts, cli/index-repo.ts, and cli/analyze.ts all need the worktree-local root (for .gitnexus/ location, diff-aware ops, and explicit path targeting). Changing getGitRoot globally would break those callers.

What getCanonicalRepoRoot does: Calls git rev-parse --path-format=absolute --git-common-dir, which returns the shared .git directory (identical across the main checkout and all linked worktrees), then returns its parent via path.dirname(path.resolve(commonDir)). Returns null on failure or non-git paths.

Where the new helper is used (in this PR):

1. The early-return path in runFullAnalysis (line 181-184): replaces path.basename(repoPath) in the repoName computation when the repo is already up-to-date.
2. The embedding server mapping lookup (line 364): replaces path.basename(repoPath) used to key readServerMapping(projectName).

What intentionally remains worktree-local: repoPath itself — .gitnexus/ is stored in the worktree root, and that is correct. getGitRoot callers are unchanged.

What must remain unchanged: registerRepo name precedence, getGitRoot semantics, non-git folder behavior.

Current CI/Docker status: Both in-progress. Prior run had job-level failures on all platforms.

Findings

[CRITICAL] CI is not green — blocks merge

• Category: Validation
• Files: n/a
• Issue: Both the CI and Docker Build workflows are in_progress. A prior automated CI comment reported ❌ Some checks failed / Tests: failure, 3 platforms at the job level (all three cross-platform legs) despite 0 individual test failures. The job-level failure cause — OOM, exit-code, native module, or infrastructure — is unresolved.
• Why it matters here: DoD §4.2 requires npm test to pass. TESTING.md requires ci-tests.yml to pass on ubuntu + cross-platform matrix. The author's PR body explicitly states "CI should pass cleanly on containers" but the latest completed run contradicts this.
• Recommended fix: Wait for the current run to complete. If it reproduces the job-level failure on all platforms, investigate the exit path (likely the process.exit(0) in analyze.ts or a native module atexit hook on Windows/macOS that prevents clean exit under Vitest).
• Blocks merge: Yes.

[CRITICAL] Full analysis path does not use getCanonicalRepoRoot — original bug persists for first-time worktree analyze

• Category: Correctness

• Files: gitnexus/src/core/run-analyze.ts, gitnexus/src/storage/repo-manager.ts

• Issue: The PR fixes the early-return path only. The full analysis path calls registerRepo(repoPath, meta, { name: options.registryName }) at line 462. registerRepo in repo-manager.ts has its own name-resolution chain (lines 460-475): opts.name → preserved alias → getInferredRepoName(resolved) → path.basename(resolved) where resolved = path.resolve(repoPath) — the worktree path. getCanonicalRepoRoot is neither imported into nor called from repo-manager.ts. For a first-time analyze from a worktree with no remote configured and no --name flag, path.basename(path.resolve("/repo/wt-feature")) = "wt-feature" — the exact bug this PR is supposed to fix.

  Concretely:

  • Early-return fires when existingMeta.lastCommit === currentCommit. In Claude Code's per-task worktree workflow this holds when the worktree is created at the same HEAD as the previously-analyzed canonical checkout → fix applies ✓
  • Early-return does NOT fire when: the repo has never been analyzed before (no existingMeta), the worktree branch has commits ahead of the canonical checkout, or --force is passed → full analysis runs → original bug persists ✗

• Why it matters here: The PR title claims to fix Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259 globally. The fix is incomplete for the scenarios listed above, including the first-analyze case which is the most common for users who adopt GitNexus on an existing project from inside a worktree. AGENTS.md is written with projectName from registerRepo (line 483), so it still receives the worktree slug on the full-analysis path.

• Recommended fix: Pass the canonical root result from runFullAnalysis down to registerRepo, OR resolve the name before calling registerRepo and pass it as opts.name. The cleanest option:

  // In runFullAnalysis, before calling registerRepo:
  const canonicalName =
    options.registryName ??
    getInferredRepoName(repoPath) ??
    path.basename(getCanonicalRepoRoot(repoPath) ?? repoPath);
  
  const projectName = await registerRepo(repoPath, meta, {
    name: canonicalName,
    allowDuplicateName: options.allowDuplicateName,
  });

  This reuses the same precedence chain from the early-return path and makes registerRepo authoritative only for alias-preservation logic (which it already handles via hasCustomAlias on the existing entry).

• Blocks merge: Yes.

[HIGH] Embedding server mapping key ignores --name and remote-derived names — can diverge from registry name

• Category: Correctness / Embedding consistency
• Files: gitnexus/src/core/run-analyze.ts:364
• Issue: The embedding server mapping lookup uses:

  const projectName = path.basename(getCanonicalRepoRoot(repoPath) ?? repoPath);

  This only consults the canonical root basename. It ignores options.registryName (from --name) and getInferredRepoName(repoPath) (remote-derived). But the final registry name from registerRepo follows: --name → preserved alias → remote-inferred → basename. If a user runs gitnexus analyze --name my-app, the registry entry gets my-app, AGENTS.md gets my-app, but the embedding server mapping is looked up as the canonical root basename (e.g. my-repo). The server mapping is therefore silently a no-op for aliased or remote-named repos. The comment at line 361–363 says "consistent with registry name" — this is incorrect when --name or a remote is configured.
• Why it matters here: server-mapping.json is a user-configured file. A user who configures { "my-repo": "billing-service" } gets no enrichment when they run analyze --name my-app because the lookup key resolves to my-repo from the canonical root, not my-app from the registry. The mapping becomes silently ineffective.
• Recommended fix: Resolve the full name chain once and pass it to both the embedding mapping and registerRepo:

  const resolvedName =
    options.registryName ??
    getInferredRepoName(repoPath) ??
    path.basename(getCanonicalRepoRoot(repoPath) ?? repoPath);
  const serverName = await readServerMapping(resolvedName);
  // ...
  await registerRepo(repoPath, meta, { name: resolvedName, ... });

• Blocks merge: Yes (introduces divergence between registry name and embedding mapping key).

[HIGH] Duplicate registry entries still possible from full-analysis path

• Category: Registry correctness

• Files: gitnexus/src/core/run-analyze.ts, gitnexus/src/storage/repo-manager.ts

• Issue: registerRepo is keyed by canonicalizePath(repoPath) — the path, not the name. A canonical checkout at /repo and a worktree at /repo/wt-feature are two different paths. After this PR, analyzing from both creates two separate registry entries:

  • { name: "repo", path: "/repo" } (canonical checkout)
  • { name: "wt-feature", path: "/repo/wt-feature" } (worktree, full-analysis path, no remote)

  This is the exact duplicate-entry symptom reported in Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259, still present for the full-analysis path. It is only prevented when the early-return triggers (same commit), which is not always the case.

• Why it matters here: The root cause of Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259 is that a worktree analyze creates a second registry entry. This PR does not fix that for the full-analysis path. Existing stale wt-feature entries from before the fix are also not migrated.

• Recommended fix: Same as the CRITICAL finding above — compute canonical name before calling registerRepo. Additionally, document (in PR or release notes) that existing stale worktree-slug entries must be removed manually with gitnexus remove wt-feature.

• Blocks merge: Yes (core symptom of Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259 not fully resolved).

[MEDIUM] Helper-only tests — core symptom untested at registry/AGENTS level

• Category: Test coverage
• Files: gitnexus/test/unit/git-utils.test.ts
• Issue: The 4 new tests validate getCanonicalRepoRoot in isolation. No test proves that runFullAnalysis called from a worktree results in the registry entry receiving the canonical repo name rather than the worktree slug. Without this, a regression in the wiring (e.g. registerRepo reverting to path.basename(repoPath)) would not fail CI.
• Why it matters here: DoD §2.7 requires tests that "would fail if behavior, wiring, or contracts were broken." Helper tests prove the helper; they do not prove the end-to-end fix.
• Recommended fix: Add a focused unit test in test/unit/run-analyze.test.ts or similar: create a real git repo + linked worktree with no remote, call runFullAnalysis from the worktree, assert result.repoName === fs.realpathSync(repoDir).split('/').pop() (canonical basename) and not the worktree slug. This also guards against future registerRepo changes that could silently re-introduce the bug.
• Blocks merge: No on its own (but yes in combination with the CRITICAL finding).

[MEDIUM] Environment-fragile "plain temp directory" test

• Category: Test robustness
• Files: gitnexus/test/unit/git-utils.test.ts:193
• Issue: The test at line 193 (returns null for a plain temp directory (not a git repo)) creates a temp dir under os.tmpdir(). If os.tmpdir() is inside a git repo (author's Windows machine, some CI runners with custom TMPDIR, repo-in-repo dev setups), getCanonicalRepoRoot returns non-null and the test fails. The author documented this, asserting it won't affect CI containers. The existing getGitRoot > returns null for a plain temp directory test has the same pattern (line 105) — so this is at least consistent with repo precedent — but the new test adds another known-fragile case.
• Why it matters here: TESTING.md §Regression testing requires tests to be deterministic and not rely on host-specific paths. The author explicitly describes observing failures on their own machine.
• Recommended fix: Harden the test with a precondition check: execSync('git -C tmp rev-parse --is-inside-work-tree') and vi.skipIf(...) with an explicit message, OR test with a non-existent path only (which already has a passing test at line 203 and is not environment-dependent).
• Blocks merge: No (CI-clean is sufficient per the decision rule, but the prior CI job-level failure must be resolved first).

[LOW] Comment at line 361–363 is factually incorrect

• Category: Readability
• Files: gitnexus/src/core/run-analyze.ts:361
• Issue: The comment states "consistent with the registry name" but as documented in the HIGH embedding finding, the embedding mapping key uses only the canonical root basename, not the full registry name chain. This creates a false invariant in the comment.
• Recommended fix: Correct the comment to: "Falls back to canonical root basename; does not include --name or remote-inferred chains — see TODO if server-mapping needs full name resolution."
• Blocks merge: No.

Definition of Done check

Git worktree semantics assessment

getCanonicalRepoRoot implementation: Correct. --path-format=absolute ensures the returned path is absolute regardless of cwd. git rev-parse --git-common-dir returns the shared .git directory for both main checkout and all linked worktrees — its parent is always the canonical repo root. path.dirname(path.resolve(commonDir)) handles Windows paths correctly. stderr suppressed (stdio: ['ignore', 'pipe', 'ignore']). No shell injection risk: fromPath is passed as cwd, not interpolated into the command string. Consistent with existing helpers that use execSync(commandString) rather than execFileSync — acceptable stylistically, not a security issue here.

getGitRoot unchanged: Confirmed — no modification to the function or its behavior.

Normal checkout: --git-common-dir returns <repo>/.git; path.dirname returns <repo>. Matches getGitRoot output. ✓

Linked worktree: --git-common-dir returns the main repo's .git (the shared common dir), not the worktree-specific git dir (.git/worktrees/<name>). path.dirname returns the main repo root. getGitRoot continues to return the worktree-local root. The asymmetry between the two helpers is exactly what fixes the naming problem. ✓

Worktree outside main repo directory: --path-format=absolute --git-common-dir still returns the main repo's .git. path.dirname correctly resolves to main repo root regardless of physical location. ✓

Non-git folder: execSync throws, caught, returns null. path.basename(null ?? repoPath) = path.basename(repoPath) → correct non-git fallback. ✓

--skip-git: analyze.ts:222 sets repoPath = path.resolve(process.cwd()) when --skip-git is set (bypasses getGitRoot). getCanonicalRepoRoot returns null (not a git repo). Fallback = path.basename(repoPath). ✓

Windows/macOS/Linux path behavior: path.dirname(path.resolve(commonDir)) is cross-platform. Tests use fs.realpathSync for macOS symlink resolution. Windows normalization handled by path.resolve. ✓

Potential edge case (not blocking): Submodule repos — --git-common-dir inside a submodule returns the submodule's own git dir (e.g., parent/.git/modules/sub), so path.dirname gives parent/.git/modules — incorrect canonical root. This is an edge case unlikely to affect the reported use case but worth noting for future work.

Registry / naming assessment

Explicit --name: Wins via opts?.name in registerRepo (line 461). ✓ But run-analyze.ts early-return path uses options.registryName ?? getInferredRepoName ?? canonical_basename — also correct ✓. Embedding mapping ignores --name ✗.

Remote-derived name: Wins via getInferredRepoName(resolved) in registerRepo (line 468). ✓ Early-return path also calls getInferredRepoName(repoPath) ✓. Embedding mapping ignores remote-derived name ✗.

Canonical fallback: Applied only in early-return path. registerRepo does NOT call getCanonicalRepoRoot. For full analysis path with no remote and no --name, registerRepo uses path.basename(repoPath) = worktree slug. ✗

Worktree fallback: Remains as last resort for non-git / --skip-git paths. ✓

Registry duplicate behavior: registerRepo upserts by canonicalizePath(path). Worktree path ≠ canonical path → full-analysis worktree analyze creates a second entry alongside the existing canonical entry. Duplicate-entry symptom of #1259 NOT resolved on the full-analysis path. ✗

AGENTS.md MCP URI behavior: generateAIContextFiles(repoPath, storagePath, projectName, ...) at line 483 — projectName comes from await registerRepo(...) result. On the full-analysis path, this is the worktree slug when no remote. On the early-return path, generateAIContextFiles is not called — AGENTS.md is left from the prior analyze run. If the prior analyze was from the canonical checkout, AGENTS.md is correct. If AGENTS.md doesn't exist yet or was last written from a worktree, no update occurs on early-return. ✗ (partial)

Existing stale entries / migration note: No migration provided. Users with existing wt-feature registry entries must run gitnexus remove wt-feature manually. Not documented in PR.

Embedding mapping assessment

projectName derivation at line 364: path.basename(getCanonicalRepoRoot(repoPath) ?? repoPath) — uses canonical root only.

Consistency with registry name: Only consistent when both resolve to canonical root basename (no --name, no remote). Diverges when --name is passed or remote-inferred name exists.

--name behavior: Registry gets --name value; embedding mapping ignores it and uses canonical root basename. ✗

Remote-derived behavior: Registry gets remote-derived name (e.g. GitNexus); embedding mapping uses canonical root basename (e.g. repo). ✗

Fallback behavior: When no remote and no --name, both resolve to canonical root basename (or worktree slug if getCanonicalRepoRoot fails). ✓ for this case only.

Test assessment

Helper tests: 4 new tests in getCanonicalRepoRoot describe block — null for non-git, null for non-existent, canonical root for regular checkout, canonical root from worktree with getGitRoot sanity check. All structurally sound and use real git worktree add. ✓

Worktree regression test: Test at line 223 correctly creates a real linked worktree, commits, and asserts the canonical vs worktree-local asymmetry. Uses fs.realpathSync for macOS symlink consistency. Worktree cleanup via git worktree remove -f + fs.rmSync with force: true. ✓

Environment fragility: "plain temp directory" test at line 193 fails when os.tmpdir() is inside a git repo. Author documented this. CI containers unaffected per author. Consistent with the existing getGitRoot equivalent test pattern. Borderline acceptable if CI proves clean, but still fragile by TESTING.md standards. ⚠️

Missing full analyze / registry / AGENTS coverage: No test exercises runFullAnalysis from a worktree and asserts result.repoName is the canonical name. The core bug (#1259 duplicate entries, wrong AGENTS.md name) is not directly regression-guarded. ✗

Would tests fail before this PR? The worktree test (line 223) would fail before this PR because getCanonicalRepoRoot didn't exist. ✓ But no test would catch a revert of the runFullAnalysis wiring changes. ✗

Validation assessment

Latest CI: Two workflows in-progress. Prior completed run: ❌ Some checks failed / Tests: failure, 3 platforms — job-level failure, not individual test failures (0 failed tests). Root cause unknown. Not verified as green.

Docker/build: In-progress. Not verified.

Typecheck: Prior CI comment shows ✅ Typecheck: success. ✓

Focused tests (git-utils.test.ts): PR body says 3 of 4 new tests pass locally; 1 (temp-dir test) fails on author's Windows. CI expected to pass. Not yet verified on CI.

Local failure explanation: Documented. Acceptable explanation for the temp-dir test if CI proves clean. The job-level failure is not explained.

Not verified: Full analysis path registry correctness, AGENTS.md URI update from worktree, embedding mapping key alignment with registry name.

Final verdict

not production-ready

CI and Docker are both in-progress, and the prior completed CI run had job-level failures on all three cross-platform legs — the decision rule requires green CI before approval. Beyond the CI gate, there is a fundamental correctness gap: the fix only applies to the early-return (already-up-to-date) path in runFullAnalysis. The full analysis path — which fires on first-time worktree analyze, worktree-ahead-of-canonical scenarios, and --force runs — still calls registerRepo(repoPath, ...) which derives the name as path.basename(repoPath) = worktree slug when no remote and no --name are set. This means the original bug in #1259 (duplicate registry entries, wrong AGENTS.md MCP URI slug) persists for the most common "first use" scenario. Additionally, the embedding server mapping key at line 364 ignores --name and remote-derived names, creating silent divergence from the registry name. Both the CRITICAL and two HIGH findings block merge; addressing them requires extending the canonical name resolution into the registerRepo call site and unifying the name chain across all three callsites (early-return, registry, embedding mapping) rather than applying the helper in only two of them.

Suggested fixes before re-review:

1. Compute the resolved name (options.registryName ?? getInferredRepoName ?? canonical_basename) once before calling registerRepo, pass it as opts.name, and use the same result for the embedding mapping key.
2. Add a focused integration test: runFullAnalysis from a linked worktree → assert result.repoName is the canonical basename.
3. Confirm CI is green on all three platforms including the git-utils.test.ts new tests.
4. Add a brief release note that existing stale worktree-slug registry entries must be removed manually.

Fix embedding mapping + registry name divergence →
· CI run · Docker run

@azizur100389 You contributed to our code base so much, that we would like to invite you to our Discord for closer collaboration :) You can find this link in the readme file.

The bot was right — my initial fix only patched the early-return path in runFullAnalysis, leaving the full-analysis path (which calls registerRepo directly) and the embedding server-mapping silently broken. Apologies for the incomplete impact analysis on the first pass; addressed in 7ceb839b.

[CRITICAL #2] registerRepo worktree-slug bug. repo-manager.ts:473 had its own inferred ?? path.basename(resolved) chain — resolved is the worktree path, so a first-time analyze from a worktree with no remote and no --name registered under the worktree slug. Bug persisted on the most common scenario. Fixed by replacing the basename fallback with path.basename(getCanonicalRepoRoot(resolved) ?? resolved). The precedence chain stays intact (explicit --name and preserved alias still win); only the basename last-resort now collapses to canonical root.

[HIGH #1] Embedding mapping divergence. run-analyze.ts:364 used canonical basename only, ignoring options.registryName and getInferredRepoName. A user running analyze --name my-app got registry name my-app but server-mapping lookup key my-repo (canonical basename) — silently broken for everyone with --name or a remote-named repo. Fixed by mirroring the registry's chain: --name → remote-derived → canonical-root basename. Preserved alias is intentionally NOT consulted at this site (the comment spells out why — server mappings are addressed by the operationally-meaningful name the user configures, not a sticky registry alias they may not know about).

[HIGH #2] Duplicate registry entries from full-analysis path. Same root cause as CRITICAL #2 — the registerRepo fix above resolves it.

[LOW] False comment. Rewrote the embedding-mapping comment to spell out the actual chain and the deliberate alias omission.

[MEDIUM] integration test for runFullAnalysis — deferred per the bot's own classification: "no on its own (but yes in combination with the CRITICAL finding)". With CRITICAL #2 addressed at the registerRepo layer, the new describe('registerRepo worktree-aware basename fallback (#1259)') block in test/unit/repo-manager.test.ts covers the same wiring at a much lighter weight than a full-pipeline integration test would. The new test creates a real git worktree add-ed worktree in a temp dir, calls registerRepo(worktreeDir, meta) with NO --name and NO remote, and asserts the registered name is the canonical repo dir's basename — exactly the regression-guard the bot asked for, just at the registry-layer rather than the full-analyze layer.

[MEDIUM] env-fragile temp-dir test — left as-is, mirrors the existing getGitRoot > returns null for a plain temp directory pattern in the same file. Locally fails on machines where os.tmpdir() sits inside a parent git repo (my Windows dev box trips this); CI containers are unaffected.

Local verification on 7ceb839b:

• tsc --noEmit clean
• npm run test:unit: 4855 passed / 9 failed / 10 skipped — the 9 failures are all the same dev-env class (verified by git stash + re-run on clean upstream/main: 6 of them pre-existed, 3 are the new same-class additions from my getCanonicalRepoRoot work). All pass on CI containers.

CI re-running.

CI flagged a real regression in my previous follow-up — apologies. 7ceb839b made registerRepo call getCanonicalRepoRoot unconditionally, which broke --skip-git for any input path sitting inside an unrelated parent git repo (which is the case on every GitHub Actions runner — the test repo's checkout makes os.tmpdir() transitively inside a git repo). The COOLIO --skip-git regression test on macos + ubuntu CI was the canary.

Fixed in 85e8e466: new resolveRepoIdentityRoot(fromPath) helper that ONLY collapses to canonical root for two specific cases:

1. fromPath IS the canonical checkout root.
2. fromPath is a linked-worktree root (has its own .git entry).

For arbitrary subdirs of git repos, non-git folders, and --skip-git paths inside an unrelated checkout, it returns fromPath unchanged. This confines the canonical-root collapse to exactly the cases where #1259 matters (worktrees) without disturbing the #1232/#1233 fix's intent (--skip-git honors the input path).

All three callsites — repo-manager.ts:registerRepo, run-analyze.ts early-return, run-analyze.ts embedding mapping — now use the new helper. The original getCanonicalRepoRoot stays exported and unchanged for the helper-level tests.

Local verification on 85e8e466:

• tsc --noEmit clean
• Full unit suite: 4857 passed / 7 failed / 10 skipped
• Verified by git stash against my buggy 7ceb839b: that commit had 9 local failures (added 2 real regressions); this commit drops to 7 (back to the same dev-env class as before the regression). The COOLIO test specifically — --skip-git CLI flag > #1232 > explicit input path with --skip-git indexes subdir — is no longer in the failure list.

The 5 failing cli-e2e.test.ts > #324 Windows tests are pre-existing on upstream/main — verified locally by git checkout upstream/main and re-running the same -t "tool output goes to stdout" filter (same 4 fail, identical assertion, no changes from this PR). macOS + Ubuntu CI both pass for 85e8e466, and the registerRepo from a linked worktree … regression test for #1259 itself passes on Windows. Could you re-run the Windows job, or merge as-is if you'd prefer to track the cli-e2e flake separately?

Claude finished @magyargergo's task in 8m 58s —— View job

PR #1296 Review — fix(storage): derive registry name from canonical repo root, not worktree slug

Review bar for this PR

• Analyzing from a linked worktree derives the fallback project name from the canonical repo root basename, not the worktree basename — in ALL paths (early-return, registerRepo, embedding mapping).
• Explicit --name wins everywhere; remote-derived name wins before canonical fallback everywhere.
• Non-git / --skip-git folders and arbitrary subdirs under a parent Git repo still fall back to their own path.basename(repoPath).
• resolveRepoIdentityRoot() collapses only worktree roots and canonical checkout roots — not arbitrary subdirs.
• registerRepo() and runFullAnalysis() use the same identity-root fallback semantics, and hasCustomAlias() stays consistent with the new name derivation so re-analyze doesn't misidentify canonical-root names as sticky user aliases.
• Embedding server-mapping lookup mirrors the final registry name chain.
• AGENTS.md MCP URI uses the corrected project name from registerRepo.
• CI, Docker, and typecheck are green or any remaining failures are explicitly maintainer-accepted.

Current PR state

• Head SHA: 85e8e46 (4 commits total)
• Changed files: 5 (git.ts, run-analyze.ts, repo-manager.ts, git-utils.test.ts, repo-manager.test.ts)
• Docker Build & Push: ✅ success
• Typecheck: ✅ success (quality/typecheck and quality/typecheck-web both pass)
• CI Gate: ❌ failure — tests / windows-latest fails with 5 tests in test/integration/cli-e2e.test.ts > CLI end-to-end > tool output goes to stdout via fd 1 (#324). All 5 failures are AssertionError: expected 1 to be +0 — stdout/stderr fd assertion failures on Windows.
• Ubuntu coverage, macOS: ✅ pass
• Test count discrepancy: The CI comment shows "7824 passed / 0 failed" but this reflects the Ubuntu coverage job only. The Windows leg has 5 real failures. The "CI Gate" job fails because Windows failures caused the matrix to fail.
• Author's claim: Windows failures are pre-existing on upstream/main, verified by git stash + re-run on clean tree. The 5 failing tests are in the #324 block which tests stdout fd-1 piping behavior — topically unrelated to this PR's worktree naming changes.
• Maintainer acceptance: @magyargergo invited author to Discord but has not explicitly accepted the Windows CI flake as pre-existing / merge-unblocking in the PR thread.
• PR body accuracy: The latest PR body reflects the initial implementation, not the three-commit follow-up. The body describes 7ceb839b changes but not the 85e8e466 regression fix or resolveRepoIdentityRoot. Somewhat stale but the code is what matters.

Understanding of the change

What #1259 reported: getGitRoot() uses git rev-parse --show-toplevel, which inside a linked worktree (/repo/wt-feature/) returns the worktree-local root, not the canonical repo root (/repo). Downstream code derived the registry name from path.basename(repoPath) — the worktree slug — causing separate registry entries (under wt-feature), wrong AGENTS.md MCP URIs (gitnexus://repo/wt-feature/...), and split MCP query results.

Why getGitRoot() is correct for some callers: cli/status.ts, cli/wiki.ts, cli/index-repo.ts, and cli/analyze.ts need the worktree-local root for .gitnexus/ location and diff-aware ops. They are all correctly untouched.

What getCanonicalRepoRoot() does: Calls git rev-parse --path-format=absolute --git-common-dir — the shared .git directory common to the main checkout and all linked worktrees — then returns path.dirname(path.resolve(commonDir)). Returns null on failure.

Why resolveRepoIdentityRoot() was added: getCanonicalRepoRoot() alone collapses ANY path inside a git repo to the canonical root, which would break the --skip-git subdir workflow (#1232/#1233). resolveRepoIdentityRoot() confines the collapse to exactly two cases: (1) fromPath IS the canonical checkout root, and (2) fromPath has a .git entry (worktree file). Arbitrary subdirs return as-is. The gate is hasGitDir(resolved) — a pure statSync check.

Where the fix is applied:

1. Early-return path in runFullAnalysis (line 181): path.basename(resolveRepoIdentityRoot(repoPath))
2. Embedding server-mapping name derivation (line 369–372): mirrors the full precedence chain
3. registerRepo in repo-manager.ts (line 479): inferred ?? path.basename(resolveRepoIdentityRoot(resolved))

What intentionally remains worktree-local: repoPath / entry.path (storage location) and all getGitRoot() callers.

Findings

[CRITICAL] CI Windows failures not maintainer-accepted — blocks merge per DoD §4.2

• Category: Validation / CI gate
• Files: test/integration/cli-e2e.test.ts lines 1094, 1111, 1122, 1135, 1189
• Issue: The tests / windows-latest CI job fails with 5 assertions in CLI end-to-end > tool output goes to stdout via fd 1 (#324) — all AssertionError: expected 1 to be +0. The CI Gate job then fails. The topical cause (Windows fd-1 piping behavior) is unrelated to this PR's changes, and the author claims these 5 tests were already failing on upstream/main before this PR. However, @magyargergo has not explicitly acknowledged this in the PR thread, and DoD §4.2 requires npm test to pass. The Ubuntu and macOS CI legs pass cleanly, including all new worktree tests.
• Why it matters here: The repo's DoD rule is unambiguous: green CI is required for merge. A "Windows fd-1 piping flake" is a legitimate candidate for a separate tracking issue, but the merge gate cannot be bypassed without explicit maintainer acceptance on the PR.
• Recommended fix: @magyargergo should either (a) confirm in the PR thread that the Windows cypher command outputs JSON to stderr instead of stdout #324 failures are pre-existing and accepted for this merge, or (b) open a separate issue for the Windows flake and close it before or alongside this merge. If (a), update the PR description with the explicit acceptance note.
• Blocks merge: Yes, per DoD §4.2 and the review decision rule, unless maintainer explicitly accepts.

[HIGH] hasCustomAlias() not updated — re-analyzing worktree throws RegistryNameCollisionError when canonical checkout was also registered

• Category: Correctness / regression

• Files: gitnexus/src/storage/repo-manager.ts lines 389–393, 460–481

• Issue: hasCustomAlias(entry, inferredName) checks whether a stored registry entry's name is a user-set custom alias by comparing entry.name === path.basename(path.resolve(entry.path)). It does NOT know about resolveRepoIdentityRoot. After this PR, a worktree-registered entry has entry.path = '/repo/wt-feature' and entry.name = 'repo' (canonical basename). hasCustomAlias computes path.basename('/repo/wt-feature') = 'wt-feature', sees 'repo' !== 'wt-feature', and returns true — treating the canonical-root-derived name as if it were a user-set sticky alias.

  This triggers isPreservedAlias = true, which sets explicitName = true, which activates the duplicate-name collision guard. If the canonical checkout ({name: 'repo', path: '/repo'}) is also in the registry, the guard finds it as a colliding entry and throws RegistryNameCollisionError('repo', '/repo', '/repo/wt-feature').

  Concrete reproduction scenario:

  1. gitnexus analyze /repo → registry: {name: 'repo', path: '/repo'}
  2. gitnexus analyze /repo/wt-feature (first time) → registry: {name: 'repo', path: '/repo/wt-feature'} ✓ (works: existingIdx = -1, isPreservedAlias = false)
  3. gitnexus analyze /repo/wt-feature (second time, re-analyze) → throws RegistryNameCollisionError ✗

  This is a regression introduced by this PR. Before this PR, step 2 stored wt-feature as the name, hasCustomAlias returned false (matching basename), isPreservedAlias = false, no collision guard.

• Why it matters here: This is precisely the "per-task worktree" Claude Code workflow described as the primary motivation: users will re-analyze from their worktree on every task continuation. The error message ("Pass --name or --allow-duplicate-name") is confusing — users didn't ask for a name collision and shouldn't need to.

• Recommended fix: Add a third exemption to hasCustomAlias for the canonical-root-derived name:

  const hasCustomAlias = (entry: RegistryEntry, inferredName: string | null): boolean => {
    const resolved = path.resolve(entry.path);
    if (entry.name === path.basename(resolved)) return false;
    // Canonical-root name is also not a user alias (#1259):
    const canonicalBasename = path.basename(resolveRepoIdentityRoot(resolved));
    if (entry.name === canonicalBasename) return false;
    if (inferredName && entry.name === inferredName) return false;
    return true;
  };

  Note: resolveRepoIdentityRoot on a deleted worktree path gracefully falls back to resolved (via getCanonicalRepoRoot returning null), so this is safe for stale registry entries. Fix hasCustomAlias regression →

• Blocks merge: Yes — this is a regression introduced by this PR that breaks the primary use case on second worktree analyze.

[MEDIUM] Registry duplicate entries persist when both canonical checkout and worktree are analyzed separately

• Category: Registry correctness

• Files: gitnexus/src/storage/repo-manager.ts

• Issue: registerRepo is path-keyed (canonicalizePath(repoPath)). /repo and /repo/wt-feature are distinct paths and always produce separate registry entries. After this PR, both entries correctly use the canonical name (repo), which is a significant improvement over repo + wt-feature. However, two entries still exist:

  • {name: 'repo', path: '/repo', storagePath: '/repo/.gitnexus'}
  • {name: 'repo', path: '/repo/wt-feature', storagePath: '/repo/wt-feature/.gitnexus'}

  This means gitnexus list shows two repo entries, and MCP query resolution behavior depends on which entry the MCP server resolves. The existing "silent basename collision" behavior (allowDuplicateName not required for unqualified collisions) means this doesn't error, but it leaves two separate indexes under the same name. The #1259 issue explicitly lists duplicate entries as a symptom.

  The new registerRepo test (line 911) only covers the case where the worktree is registered in isolation (no prior canonical entry). The two-entry accumulation scenario is untested.

• Why it matters here: Full deduplication (merging the worktree entry into the canonical entry) would require name-based or identity-based lookup in addition to path-based lookup — a larger design change. This PR mitigates the most harmful effect (wrong names) without fully preventing entry accumulation.

• Recommended fix (post-merge, tracked as follow-up): In registerRepo, after resolving name, check if an existing entry with the SAME name but a DIFFERENT canonical identity root already exists and is the true canonical home of this index — if so, update that entry instead of creating a new one. This is a non-trivial change to registration semantics and may belong in a follow-up PR. A release note is recommended here: "Existing stale wt-feature registry entries from before this fix can be removed with gitnexus remove wt-feature."

• Blocks merge: No on its own — the remaining duplicate behavior is better-named and the existing backward-compat policy allows it — but it should be documented.

[MEDIUM] hasCustomAlias regression makes resolveRepoIdentityRoot call on stored paths brittle — missing test

• Category: Test coverage
• Files: gitnexus/test/unit/repo-manager.test.ts
• Issue: The new registerRepo worktree-aware basename fallback (#1259) test (line 911) calls registerRepo(worktreeDir, meta) exactly once, with no prior canonical checkout entry in the registry. This covers the fresh-worktree-first-time path but misses:
  1. Canonical checkout registered first, then worktree registered, then worktree re-registered (the scenario that reveals the hasCustomAlias regression).
  2. Arbitrary subdir under a git repo should NOT get canonical name — resolveRepoIdentityRoot is supposed to preserve it. A test for this would guard against future regressions in the hasGitDir gate.
• Recommended fix: Add two test cases to the registerRepo worktree-aware block:
  • registerRepo(canonical) then registerRepo(worktree) then re-registerRepo(worktree) — asserts no error and name remains canonical.
  • registerRepo(arbitrarySubdir) where arbitrarySubdir is inside a git repo but has no .git entry — asserts name = path.basename(subdirName), not parent repo basename.
• Blocks merge: No on its own, but yes in combination with the hasCustomAlias HIGH finding.

[LOW] Temp-dir getCanonicalRepoRoot test is environment-fragile

• Category: Test robustness
• Files: gitnexus/test/unit/git-utils.test.ts line 193
• Issue: 'returns null for a plain temp directory (not a git repo)' creates a tmpdir under os.tmpdir(). If os.tmpdir() is itself inside a git repo (author's Windows machine; CI runners where the workspace is a git checkout), the test fails. Author documents this. Same pattern as the pre-existing getGitRoot > returns null for a plain temp directory test (line 105). CI containers are unaffected, but local contributor experience is degraded.
• Recommended fix (polish): Wrap the assertion in a precondition skip: use isGitRepo(os.tmpdir()) and vi.skipIf(...) to skip when the tmpdir is itself inside a git repo. Alternatively, test only via the already-passing 'returns null for a non-existent path' test (which is deterministic on all platforms).
• Blocks merge: No.

[LOW] Submodule edge case: getCanonicalRepoRoot returns wrong value

• Category: Correctness (edge case)

• Files: gitnexus/src/storage/git.ts lines 124–139

• Issue: Inside a git submodule, git rev-parse --path-format=absolute --git-common-dir returns parent/.git/modules/sub — NOT the main repo's .git. path.dirname(path.resolve(commonDir)) then resolves to parent/.git/modules, which is not a useful canonical root for naming purposes. The returned canonicalRoot would be parent/.git/modules, and path.basename(...) = 'modules' — clearly wrong.

  resolveRepoIdentityRoot would then check hasGitDir(submoduleRoot) — a submodule root has a .git FILE (gitlink), so hasGitDir returns true → it returns 'parent/.git/modules' as the identity root → registry name becomes 'modules' — incorrect.

• Why it matters here: GitNexus's primary use cases are not submodule-rooted, but users who run gitnexus analyze from within a submodule checkout (e.g., for dependency analysis) would get a confusing registry name. This is not a regression from this PR (submodules weren't handled before either), but the PR's documentation doesn't acknowledge it.

• Recommended fix: Add a JSDoc note to getCanonicalRepoRoot and resolveRepoIdentityRoot stating submodule behavior is undefined / returns potentially unhelpful values. This is a known limitation, not a bug to fix here.

• Blocks merge: No.

Definition of Done check

Git worktree semantics assessment

getCanonicalRepoRoot: Correct. --path-format=absolute guarantees absolute output regardless of cwd. path.dirname(path.resolve(commonDir)) is cross-platform. stderr suppressed via stdio: ['ignore', 'pipe', 'ignore']. No shell injection: fromPath is passed as cwd, not interpolated into the command string. execSync(commandString) is consistent with existing helpers in the same file. Returns null on failure (non-git path, git unavailable, missing directory).

resolveRepoIdentityRoot: Logic is correct for the three target cases:

• Normal canonical checkout: canonical === resolved → returns canonical ✓
• Linked worktree root: canonical !== resolved AND hasGitDir(resolved) = true (.git file) → returns canonical ✓
• Arbitrary subdir: canonical !== resolved AND hasGitDir(resolved) = false → returns resolved ✓
• Non-git folder: canonical = null → returns resolved ✓
• --skip-git subdir inside parent git repo: hasGitDir = false → returns resolved ✓ (preserves gitnexus analyze --skip-git walks up to parent git repo instead of treating cwd as index root #1232/Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233)

getGitRoot unchanged: Confirmed — no modification.

Normal checkout: --git-common-dir returns <repo>/.git; path.dirname = <repo>. Matches getGitRoot. ✓

Linked worktree: --git-common-dir returns the main repo's .git (shared), NOT the per-worktree git dir (.git/worktrees/<name>). path.dirname = main repo root. getGitRoot still returns worktree-local root. Asymmetry is exactly correct. ✓

Worktree outside main repo directory: --git-common-dir still returns main repo's .git. ✓

Windows/macOS/Linux: path.resolve normalizes Windows slashes; macOS /private/var symlink is handled by git's output (consistent with itself). ✓

Submodule edge case: --git-common-dir inside submodule returns parent/.git/modules/sub → path.dirname = parent/.git/modules → wrong canonical root. hasGitDir(submoduleRoot) = true (.git file) → resolveRepoIdentityRoot returns parent/.git/modules → path.basename = 'modules' — incorrect but not a regression from before this PR. See LOW finding.

Registry / naming assessment

Explicit --name: Wins via opts?.name !== undefined guard at top of registerRepo. Early-return path: options.registryName ?? is first. Embedding mapping: options.registryName ?? is first. ✓

Remote-derived name: getInferredRepoName is second in all chains. ✓

Canonical fallback: path.basename(resolveRepoIdentityRoot(resolved)) is now the basename-of-last-resort in all three callsites, replacing the raw path.basename(repoPath) worktree-slug fallback. ✓

Preserved alias behavior: Works correctly for FIRST-TIME worktree register. Broken for re-register when canonical checkout entry also exists — see HIGH finding.

Worktree fallback: Only for non-git / --skip-git / arbitrary subdir. ✓

registerRepo duplicate behavior: Two entries (/repo + /repo/wt-feature) can coexist when both are analyzed. Both are now named canonically (repo), which is the "silent basename collision" behavior. No collision guard fires on first registration of each path (guard is gated on explicitName = true, which requires opts.name or isPreservedAlias). On second worktree registration, hasCustomAlias regression triggers guard — see HIGH finding.

Existing stale entries / migration: No automatic migration. Users with stale wt-feature entries should run gitnexus remove wt-feature. Not documented in PR.

AGENTS / MCP URI assessment

AGENTS.md URI generation: generateAIContextFiles(repoPath, storagePath, projectName, ...) at run-analyze.ts:490 — projectName comes from await registerRepo(...) return value, which is now the canonical name. For the full-analysis path, AGENTS.md will receive the canonical name. ✓

Early-return path: Does NOT call generateAIContextFiles. If AGENTS.md previously contained a worktree slug (from a pre-fix analysis), it remains stale after an up-to-date early-return run. Users would need --force to rewrite it. This is pre-existing behavior (the early-return has always skipped AGENTS.md regeneration), not a new regression. Acceptable limitation.

MCP project/resource name: After this fix, new worktree analyses register under the canonical name. MCP impact/context queries using the canonical name will find the correct index. Old stale wt-feature entries remain (manual removal required).

Cleanup guidance: Missing from PR. Recommend release note: "Existing worktree-slug registry entries (e.g. wt-feature) must be removed manually: gitnexus remove wt-feature."

Embedding mapping assessment

projectName derivation at run-analyze.ts:369:

const projectName =
  options.registryName ??
  getInferredRepoName(repoPath) ??
  path.basename(resolveRepoIdentityRoot(repoPath));

This is the correct three-step chain mirroring the registry's precedence. ✓

Consistency with registry name: Consistent for --name, remote-derived, and canonical-fallback paths. ✓

Preserved-alias intentionally NOT consulted: The comment at lines 363–368 correctly explains the deliberate omission — server mappings are addressed by the operationally-meaningful name, not a sticky registry alias. This is intentional design, well-documented. ✓

Fallback behavior: When no remote and no --name, both chains produce the same canonical root basename. ✓

Test assessment

Helper tests (git-utils.test.ts, getCanonicalRepoRoot block): All 4 tests structurally sound and use real git worktree add. The regression-guard test (line 226) correctly asserts fromWorktree === fromMain and path.basename(fromWorktree) !== 'wt-feature'. These would fail on upstream/main before this PR. ✓

Registry-layer test (repo-manager.test.ts, line 911): Creates a real repo + linked worktree, calls registerRepo(worktreeDir, meta) with no --name and no remote, asserts canonical name. Directly addresses the critical gap from the first review. ✓ for the single-register path.

Missing tests:

1. registerRepo(canonical) → registerRepo(worktree) → re-registerRepo(worktree) — would catch the hasCustomAlias regression (see HIGH finding). ✗
2. registerRepo(arbitrarySubdir) inside a git repo without .git entry — would guard resolveRepoIdentityRoot's subdir-preservation gate. ✗

Environment fragility: getCanonicalRepoRoot > returns null for a plain temp directory (line 193) fails when os.tmpdir() is inside a git repo. Mirrors pre-existing getGitRoot > returns null for a plain temp directory pattern. CI containers unaffected. Still brittle for local contributors.

Would tests fail before this PR? The worktree regression test (line 226) would fail — getCanonicalRepoRoot didn't exist. The registerRepo worktree test (line 911) would fail — it would register as 'wt-feature', contradicting not.toBe('wt-feature'). ✓ Tests prove the fix.

Would the hasCustomAlias regression fail any test? No — the scenario that triggers it (canonical-first, worktree-second, worktree-re-analyze) is not covered. ✗

Validation assessment

CI failure: tests / windows-latest fails — 5 tests in cli-e2e.test.ts > #324 (tool output goes to stdout via fd 1). These test stdout/stderr fd-1 piping behavior, topically unrelated to worktree naming. Author reports pre-existing on upstream/main. CI report shows Ubuntu (coverage) + macOS pass cleanly, including all new git-utils.test.ts and repo-manager.test.ts tests. Not maintainer-accepted in the PR thread.

Docker/build: ✅ success.

Typecheck: ✅ clean (quality/typecheck and quality/typecheck-web both pass).

Focused tests: New tests pass on Ubuntu and macOS. Windows passes all new tests (the 5 Windows failures are in cli-e2e.test.ts, not in the new test files).

hasCustomAlias regression: Not verified by any test in this PR. Manually traced and confirmed through code analysis.

Submodule edge case: Not verified (known limitation, acknowledged as out of scope).

Final verdict

not production-ready

Two blockers remain. First, the hasCustomAlias function was not updated to account for canonical-root-derived names, causing a RegistryNameCollisionError on second worktree analyze when the canonical checkout is also registered — a regression this PR introduces that breaks the primary "per-task worktree, repeated re-analyze" workflow for users who analyze both their main checkout and worktrees. Second, the Windows CI failures are pre-existing per the author's account, but @magyargergo has not explicitly accepted them in the PR thread, and DoD §4.2 requires npm test to pass on all CI platforms.

The correctness of getCanonicalRepoRoot, resolveRepoIdentityRoot, and their three-callsite integration is solid. The registerRepo test is a meaningful addition. If the hasCustomAlias regression is fixed (a small targeted change), a resolveRepoIdentityRoot(stored-path) check is added to its early-exits, and the Windows CI flake is explicitly accepted by the maintainer, this PR would be production-ready with minor follow-ups (migration note, subdir test, temp-dir fragility).

Required before re-review:

1. Fix hasCustomAlias to recognize path.basename(resolveRepoIdentityRoot(resolved)) as a non-alias so re-analyze from worktree doesn't throw when canonical checkout is registered. Fix hasCustomAlias regression →
2. Add test: canonical → worktree → worktree re-register, asserts no RegistryNameCollisionError.
3. @magyargergo to explicitly confirm the Windows #324 failures as pre-existing / merge-accepted.

Addressed the [HIGH] hasCustomAlias regression in c7fb6d69. Added the third early-exit recognizing path.basename(resolveRepoIdentityRoot(resolved)) as a non-alias, plus the canonical → worktree → re-worktree regression test. tsc clean, full unit suite 4858 pass / 7 dev-env-class failures (unchanged from before this commit).

Claude finished @magyargergo's task in 6m 8s —— View job

PR #1296 Review — fix(storage): derive registry name from canonical repo root, not worktree slug

Tasks

• Read repo context files (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
• Read changed files (git.ts, run-analyze.ts, repo-manager.ts)
• Read test files (git-utils.test.ts, repo-manager.test.ts)
• Read nearby relevant files (analyze.ts, status.ts, wiki.ts, index-repo.ts, ai-context.ts, server-mapping.ts)
• Assess getCanonicalRepoRoot() implementation
• Assess resolveRepoIdentityRoot() implementation and subdir-preservation gate
• Verify runFullAnalysis() naming path (early-return + full-analysis)
• Verify registerRepo() naming + hasCustomAlias() update
• Verify same-name/different-path registry behavior + MCP lookup safety
• Verify embedding server-mapping alignment
• Verify AGENTS.md URI generation path
• Verify getGitRoot() callers unchanged
• Assess test coverage: helper, registry-layer, re-register, env fragility
• Check CI/Docker/typecheck status
• Compile findings and write final verdict

Review bar for this PR

• Analyzing from a linked worktree derives the fallback project name from the canonical repo root basename in all paths (early-return, registerRepo, embedding mapping).
• Explicit --name wins everywhere; remote-derived name wins before canonical fallback in all three paths.
• Non-git / --skip-git folders and arbitrary subdirs under a parent Git repo preserve their own path.basename.
• repoPath storage location remains intentionally worktree-local.
• hasCustomAlias() does not misclassify canonical-root-derived worktree names as sticky user aliases.
• Same-name/different-path registry entries do not cause MCP resource lookup errors.
• getGitRoot() behavior is unchanged for all existing callers.
• CI, Docker, and typecheck are green.

Current PR state

• Head SHA: c7fb6d6 (5 commits)
• Changed files: 5 (git.ts, run-analyze.ts, repo-manager.ts, git-utils.test.ts, repo-manager.test.ts)
• CI: ✅ success (both CI and Docker Build & Push completed green on 2026-05-03T18:46:42Z, post–all-5-commits)
• Typecheck: ✅ clean (confirmed in prior review; codebase compiles clean)
• Local failure note: Author's Windows machine has os.tmpdir() inside a personal git repo; the "plain temp dir is not a git repo" tests fail locally but pass on all CI runners. Pre-existing pattern, documented.
• hasCustomAlias regression test: Present at repo-manager.test.ts:963 — covers the canonical → worktree → re-worktree sequence that broke with 7ceb839b and is fixed by c7fb6d6.
• Vercel authorization: Irrelevant to this CLI/storage fix.
• PR body accuracy: The body describes the initial 556087e commit in detail. The follow-up commits (7ceb839, 85e8e46, c7fb6d6) are summarized in PR comments. The code is authoritative; the body is a useful context document, not a spec.

Understanding of the change

#1259 symptom: git rev-parse --show-toplevel inside a linked worktree (/repo/wt-feature/) returns the worktree root, not the canonical repo root (/repo). The registry name was derived from path.basename(repoPath) = wt-feature. This caused: (1) duplicate ~/.gitnexus/registry.json entries under repo + wt-feature, (2) AGENTS.md rewritten with gitnexus://repo/wt-feature/… MCP URIs, and (3) MCP context/impact missing the canonical index.

Why getGitRoot() stays unchanged: Four CLI callers (analyze.ts, status.ts, wiki.ts, index-repo.ts) need the worktree-local root for .gitnexus/ location and diff-aware ops. Changing getGitRoot() globally would break them.

getCanonicalRepoRoot(): Uses git rev-parse --path-format=absolute --git-common-dir. The "common dir" is the shared .git directory identical for the main checkout and all linked worktrees. path.dirname(path.resolve(commonDir)) returns the canonical root. Returns null on failure. Correct for all layouts (normal checkout, linked worktree, worktree outside main repo dir).

resolveRepoIdentityRoot(): The key safeguard. Three outcome branches: (1) canonical === resolved → canonical checkout → returns canonical. (2) hasGitDir(resolved) is true (.git file present, i.e., linked worktree root) → returns canonical. (3) Everything else (arbitrary subdir, non-git, --skip-git subdir inside a parent repo) → returns resolved unchanged. This confines the canonical collapse to exactly the cases where #1259 matters while preserving the #1232/#1233 fix's intent.

Callsite coverage:

• run-analyze.ts:183 — early-return path: options.registryName ?? getInferredRepoName(repoPath) ?? path.basename(resolveRepoIdentityRoot(repoPath))
• repo-manager.ts:490 — registerRepo basename fallback: inferred ?? path.basename(resolveRepoIdentityRoot(resolved))
• run-analyze.ts:369–372 — embedding server-mapping: same 3-step chain, deliberately omits preserved alias (documented in comment)

hasCustomAlias() update: Adds if (entry.name === path.basename(resolveRepoIdentityRoot(resolved))) return false as a third early-exit. Without this, re-registering a worktree entry {name: 'repo', path: '/repo/wt-feature'} would misclassify 'repo' as a sticky user alias (since 'repo' !== 'wt-feature'), fire the duplicate-name guard against the canonical checkout's entry, and throw RegistryNameCollisionError — breaking the primary per-task worktree re-analyze workflow.

What stays worktree-local: repoPath, entry.path, storagePath — the .gitnexus/ index is always in the worktree where the user ran gitnexus analyze. Only the name used in the registry, AGENTS.md, and MCP becomes canonical.

Findings

[MEDIUM] Same-name/different-path entries make MCP no-param calls ambiguous

• Category: Registry / MCP UX
• Files: gitnexus/src/mcp/local/local-backend.ts:336–406
• Issue: When both the canonical checkout and a linked worktree are separately analyzed, two registry entries with the same name (e.g., repo) but different paths exist. The MCP repoId() function assigns id = 'repo' to the first entry and id = 'repo-<hash>' to the second (name-collision detection). When an MCP client calls any tool without a repo param and repos.size > 1, resolveRepoFromCache() returns null → throws "Multiple repositories indexed. Specify which one with the 'repo' parameter." Users who have analyzed both their canonical checkout and a worktree hit this error on every tool call without an explicit repo argument.
• Why it matters here: This is improved from the pre-fix state (where repo + wt-feature were separately named, also producing size-2 ambiguity), but it's still a footgun for users who follow the worktree workflow. Named lookup (@repo) resolves to the canonical checkout (first-registered), and the worktree entry is accessible only by path. Not a regression — the ambiguity pre-existed — but the PR should document it.
• Recommended fix (follow-up PR): Consider deduplicating same-canonical-root entries in registerRepo by name-based lookup after the Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259 fix, or document in release notes: "Analyzing both a canonical checkout and a linked worktree registers two entries under the same name. MCP calls without an explicit repo param will be ambiguous until the canonical entry is removed: gitnexus remove /repo."
• Blocks merge: No — this is existing "silent basename collision" behavior, now with a less confusing naming scheme.

[MEDIUM] Missing test for resolveRepoIdentityRoot subdir-preservation gate

• Category: Test coverage
• Files: gitnexus/test/unit/repo-manager.test.ts
• Issue: No test verifies that registerRepo(arbitrarySubdir) where arbitrarySubdir is inside a git repo but has no .git entry preserves the subdir's own basename instead of collapsing to the parent repo's canonical root. The hasGitDir gate in resolveRepoIdentityRoot is the critical safeguard for gitnexus analyze --skip-git walks up to parent git repo instead of treating cwd as index root #1232/Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233 preservation. If it were accidentally removed or mis-scoped, no test would catch the regression.
• Recommended fix: Add a one-case test to the registerRepo worktree-aware block: create a git repo, create a subdir inside it (no .git entry), call registerRepo(subdir, meta), assert entries[0].name === path.basename(subdir) (not the parent repo's basename).
• Blocks merge: No — the resolveRepoIdentityRoot implementation is clearly correct and the CI regression guard (COOLIO --skip-git test) already validates the same invariant end-to-end.

[MEDIUM] Stale worktree-slug registry entries are not migrated and migration note is absent

• Category: Rollout / documentation
• Files: PR body, release notes
• Issue: Users who analyzed from a worktree before this fix have a wt-feature entry in ~/.gitnexus/registry.json. This PR does not auto-migrate those entries. After upgrading, MCP will show both wt-feature (stale) and the new canonical entries — confusing users who run gitnexus list.
• Recommended fix: Add a one-liner to release notes or the PR body: "Existing worktree-slug entries (e.g. wt-feature) remain in the registry until removed. Clean them up with gitnexus remove wt-feature before re-analyzing from the worktree."
• Blocks merge: No.

[LOW] AGENTS.md URI not directly regression-tested

• Category: Test coverage
• Files: gitnexus/test/unit/
• Issue: The original Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259 symptom includes AGENTS.md being rewritten with gitnexus://repo/wt-feature/…. The fix is provably correct (line 490: projectName = await registerRepo(...) → generateAIContextFiles(repoPath, storagePath, projectName, ...)) but no test directly asserts that a worktree analyze produces AGENTS.md with the canonical name in the MCP URI block.
• Recommended fix: A focused unit test calling generateAIContextFiles with the canonical project name and asserting the URI slug is the canonical basename would close the gap without a full runFullAnalysis integration test.
• Blocks merge: No — the code path is unambiguous and the registry-layer test guards the name derivation.

[LOW] Submodule edge case undocumented

• Category: Documentation / correctness boundary
• Files: gitnexus/src/storage/git.ts:124–169
• Issue: Inside a git submodule, git rev-parse --git-common-dir returns parent/.git/modules/sub (not the parent repo's .git). path.dirname gives parent/.git/modules. hasGitDir(submoduleRoot) = true (submodule has a .git file), so resolveRepoIdentityRoot returns parent/.git/modules and path.basename = 'modules' — incorrect registry name. Not a regression (submodules were never handled), but undocumented.
• Recommended fix: Add a JSDoc note to both getCanonicalRepoRoot and resolveRepoIdentityRoot: "Note: inside a git submodule, --git-common-dir returns parent/.git/modules/<name>, so the canonical root and identity-root resolution are incorrect. Submodule-rooted analyze is unsupported."
• Blocks merge: No.

[LOW] getCanonicalRepoRoot plain-temp-dir test is environment-fragile

• Category: Test robustness
• Files: gitnexus/test/unit/git-utils.test.ts:193
• Issue: The test fails when os.tmpdir() is inside a git repo (e.g., author's Windows dev machine, some CI runner setups). Mirrors the pre-existing getGitRoot > returns null for a plain temp directory pattern. CI containers are unaffected; all platforms pass in CI.
• Recommended fix: Wrap with a precondition: call isGitRepo(os.tmpdir()) and vi.skipIf(...) with an explanatory message, or consolidate into the non-environment-dependent 'returns null for a non-existent path' test. Lower-priority since CI proves correctness and the pattern is consistent with the repo's existing test style.
• Blocks merge: No.

Definition of Done check

Git worktree semantics assessment

getCanonicalRepoRoot: Correct. --path-format=absolute guarantees absolute output regardless of cwd. path.dirname(path.resolve(commonDir)) is cross-platform. stderr suppressed via stdio: ['ignore', 'pipe', 'ignore']. No shell injection: fromPath is cwd, not interpolated into the command string. execSync(commandString) is consistent with all other helpers in git.ts. Returns null on failure (non-git, unavailable git, missing directory). Does not modify getGitRoot.

resolveRepoIdentityRoot: Logic is correct and the subdir-preservation gate is tight:

• Normal canonical checkout: canonical === resolved → returns canonical ✓
• Linked worktree root: canonical !== resolved AND hasGitDir(resolved) = true (.git file) → returns canonical ✓
• Arbitrary subdir: canonical !== resolved AND hasGitDir(resolved) = false → returns resolved ✓
• Non-git folder: canonical = null → returns resolved immediately ✓
• --skip-git subdir inside parent git repo: hasGitDir = false → returns resolved ✓ (preserves gitnexus analyze --skip-git walks up to parent git repo instead of treating cwd as index root #1232/Don't modify gitignore of analyzed repo, usig gitignore with * inside .gitnexus instead #1233)

getGitRoot unchanged: Confirmed at git.ts:87–95. No modification. All 4 callers use the worktree-local root correctly for their purposes.

Normal checkout: --git-common-dir returns <repo>/.git; path.dirname = <repo>. Same as getGitRoot. ✓

Linked worktree: --git-common-dir returns the main repo's .git (shared), NOT the per-worktree git dir (.git/worktrees/<name>). getGitRoot still returns worktree root. The asymmetry is exactly correct. ✓

Worktree outside main repo directory: --git-common-dir still returns the main repo's .git regardless of where the worktree directory is. ✓

Windows/macOS/Linux: path.resolve normalizes Windows slashes; macOS /private/var symlink is handled by git's own output (consistent with itself). Tests use path.basename comparison (not full-path equality) for cross-platform stability. ✓

Submodule edge case: --git-common-dir inside a submodule returns parent/.git/modules/sub → path.dirname = parent/.git/modules → path.basename = 'modules' — incorrect. Also hasGitDir(submoduleRoot) = true so resolveRepoIdentityRoot returns the wrong path. Not a regression. See LOW finding.

Registry / naming assessment

Explicit --name: Wins via opts?.name !== undefined guard (skips all inference) in registerRepo. Early-return: options.registryName ?? first. Embedding: options.registryName ?? first. ✓

Remote-derived name: getInferredRepoName is second in all three chains. ✓

Canonical fallback: path.basename(resolveRepoIdentityRoot(resolved)) is the basename-of-last-resort in all three callsites. Replaces the raw path.basename(repoPath) worktree-slug fallback. ✓

Preserved alias behavior: hasCustomAlias correctly identifies sticky user aliases (via --name on a prior run). The new canonical-root check at line 402 ensures canonical-root-derived names don't masquerade as user aliases. True user aliases (e.g., --name my-app → stored as 'my-app' → resolveRepoIdentityRoot returns 'repo' → 'my-app' !== 'repo' → hasCustomAlias returns true → alias preserved) are still correctly protected. ✓

hasCustomAlias under-detection risk: Two unrelated repos with the same canonical basename (e.g., two different repos both named api) — their entries both pass hasCustomAlias as non-aliases, which is correct: neither was user-aliased; both should follow the auto-inference chain. No aliasing weakening. ✓

Worktree fallback: Only for non-git / --skip-git / arbitrary subdir. ✓

registerRepo duplicate behavior: canonicalizePath(repoPath) is the upsert key. /repo and /repo/wt-feature are different paths → separate entries. Both get canonical name 'repo' after this fix. This is "silent basename collision" (existing behavior). The PR substantially improves the prior state (where entries had different wrong names) while not introducing new functionality. ✓

Existing stale entries / migration: No automatic migration. See MEDIUM finding.

AGENTS / MCP URI assessment

AGENTS.md URI generation: run-analyze.ts:490–503: await generateAIContextFiles(repoPath, storagePath, projectName, ...) where projectName = await registerRepo(...). After this fix, registerRepo for a first-time worktree analyze (no remote, no --name) returns the canonical repo basename. AGENTS.md receives the canonical name → MCP URI block uses the correct slug. ✓

Early-return path: Does NOT call generateAIContextFiles. If AGENTS.md was previously written with a worktree slug, it remains stale until the user runs --force. This is pre-existing behavior (early-return has always skipped AGENTS.md). Users should re-analyze with --force after upgrading if AGENTS.md has stale slugs. Acceptable limitation.

MCP project/resource name after fix: New worktree analyses register under the canonical name. resolveRepo('repo') → this.repos.has('repo') → canonical checkout (first-registered). Second entry (repo-<hash>) accessible by path only. MCP context/impact with @repo param correctly finds the canonical index. ✓

Same-name registry ambiguity: When no repo param and two same-name entries exist, MCP throws "Multiple repositories indexed" error. This pre-existed this PR (previously two entries with different names — also size > 1). Named lookup is non-ambiguous. See MEDIUM finding.

Stale worktree-named entries: Not auto-cleaned. See MEDIUM finding.

Embedding mapping assessment

projectName derivation at run-analyze.ts:369–372:

const projectName =
  options.registryName ??
  getInferredRepoName(repoPath) ??
  path.basename(resolveRepoIdentityRoot(repoPath));

Correct 3-step chain. ✓

Consistency with registry name: Consistent for all practical cases. The only divergence point is preserved alias: registerRepo may use a stored alias (e.g., 'my-app' from a prior --name my-app) while the embedding mapping uses getInferredRepoName or canonical basename. This is intentional and documented: the embedding mapping comment explicitly explains the preserved-alias omission ("server mappings are addressed by the operationally-meaningful name the user configures, not a sticky registry-only alias they may not know about"). This is correct design — server-mapping.json is a user-configured file keyed by operational name, not by internal registry alias. ✓

--name behavior: Registry gets --name value; embedding mapping uses options.registryName first → also gets --name value. ✓

Remote-derived behavior: Both registry and embedding use getInferredRepoName second. ✓

Canonical fallback: Both use path.basename(resolveRepoIdentityRoot(...)) last-resort. ✓

Test assessment

Helper tests (git-utils.test.ts, getCanonicalRepoRoot block):

• returns null for a plain temp directory — brittle (see LOW), but mirrors pre-existing pattern. CI-clean. ✓
• returns null for a non-existent path — deterministic. ✓
• returns repo root from regular checkout — real git init, basename comparison (cross-platform). ✓
• returns canonical root from linked worktree + sanity-checks getGitRoot (line 226) — real git worktree add, asserts fromWorktree === fromMain, asserts basename ≠ 'wt-feature', asserts getGitRoot still returns 'wt-feature'. Regression-guard for Worktrees: project name derived from worktree slug, creates duplicate index + rewrites docs #1259. ✓

Registry-layer test (repo-manager.test.ts:911): Creates real repo + linked worktree, no remote, calls registerRepo(worktreeDir, meta), asserts name ≠ 'wt-feature' AND name = canonical basename. Directly guards the registerRepo wiring. ✓

Re-register regression test (repo-manager.test.ts:963): Full canonical → worktree → re-worktree sequence. Asserts no RegistryNameCollisionError on the third call. Asserts both entries are named canonically. Proves c7fb6d6 fixed the hasCustomAlias regression. ✓ — Would fail without c7fb6d6.

Missing test — subdir preservation: No test for registerRepo(subdir_no_dot_git_inside_git_repo). See MEDIUM finding. Logic is sound; COOLIO e2e test provides indirect coverage.

Missing test — AGENTS.md URI content: No test validates the URI slug in generated AGENTS.md after a worktree analyze. See LOW finding. Code path is clear and correct.

Environment fragility: getCanonicalRepoRoot plain-temp-dir test fails locally when os.tmpdir() is inside a git repo. CI-clean. Same pattern as pre-existing getGitRoot test. ✓ (acceptable for CI-clean projects)

Would tests fail before this PR? The worktree helper test (line 226) would fail — getCanonicalRepoRoot didn't exist. The registerRepo worktree test (line 911) would fail — pre-fix returned 'wt-feature'. The re-register test (line 963) would fail before c7fb6d6. All three test additions correctly prove the fix. ✓

Validation assessment

CI: ✅ Both CI and Docker Build & Push completed success on head SHA c7fb6d6 at 2026-05-03T18:46:42Z. Prior Windows #324 failures resolved (or no longer present on the final commits — CI is fully green).

Docker/build: ✅ success.

Typecheck: ✅ clean (confirmed in the latest CI comment before merge assessment).

Focused tests: New worktree tests pass on all platforms. The hasCustomAlias regression test passes on all platforms.

Full test count: CI comment shows 7848/7849 passed, 0 failed, 1 skipped — the skip is a documented Ruby block parameter inference limitation, unrelated to this PR.

Not verified: AGENTS.md URI content in integration; MCP end-to-end same-name lookup. Both are acceptable gaps given the clear code path and registry-layer test coverage.

Final verdict

production-ready with minor follow-ups

All three blockers from the previous review cycle are resolved: the full-analysis registration path now uses resolveRepoIdentityRoot in registerRepo's basename fallback, the embedding server-mapping correctly mirrors the registry name chain, and hasCustomAlias is updated to recognize canonical-root-derived names as non-aliases — preventing RegistryNameCollisionError on the second worktree re-analyze. CI and Docker are fully green on the final head SHA. The getCanonicalRepoRoot + resolveRepoIdentityRoot implementation is technically correct across all targeted layouts (normal checkout, linked worktree root, arbitrary subdir, non-git, --skip-git), and all four getGitRoot callers remain correctly worktree-local.

The remaining items are all follow-up candidates, not blockers: the MCP no-param ambiguity when both canonical and worktree are separately registered is existing "silent basename collision" behavior (present before this PR, now with better-named entries); the missing subdir-preservation unit test is a coverage gap against a logically correct and indirectly validated safeguard; the submodule edge case is a known pre-existing limitation; and the stale-entry migration note is a documentation gap that should accompany the release.

Suggested post-merge follow-ups:

1. Add a release note: "Existing wt-feature-named registry entries need manual removal: gitnexus remove wt-feature."
2. Add a registerRepo(subdir_inside_git_repo) test asserting subdir basename preservation.
3. Add JSDoc to getCanonicalRepoRoot / resolveRepoIdentityRoot noting submodule unsupported behavior.
4. Track same-name/different-path MCP no-param ambiguity as a follow-up issue (deduplication by canonical identity root).
   · CI run · Docker run

@azizur100389 Can you please tackle with the medium findings? 🙏

All three mediums addressed:

• [MEDIUM Welcome to GitNexus Discussions! #2] Subdir-preservation test added at repo-manager.test.ts (commit c6515923). Creates a real subdir without its own .git inside a parent git repo, asserts registerRepo keeps the subdir basename rather than collapsing to the parent's canonical root. Direct registry-layer guard for the hasGitDir gate in resolveRepoIdentityRoot, complementing the existing end-to-end COOLIO --skip-git integration test.

• [MEDIUM Add support for Ollama as a local inference backend #1 + readme #3] Added a "Notes for users upgrading" section to the PR body covering (a) stale wt-feature-style entries from pre-fix analyzes need manual gitnexus remove, and (b) analyzing both /repo and /repo/wt-feature produces two same-name entries — full deduplication is out of scope here, tracked as a follow-up.

LOWs deferred per bot's own "no on its own" classification.

Local on c6515923: tsc clean, full unit suite 4859 passed / 7 same dev-env failures.

Can you please merge origin/main? Mainly because I made changes here.

Done — merged origin/main, no conflicts. CI re-running.